Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[bitnami/keycloak] Add support for ipv6 #40224

Merged
merged 6 commits into from
Jul 19, 2023
Merged

[bitnami/keycloak] Add support for ipv6 #40224

merged 6 commits into from
Jul 19, 2023

Conversation

covert8
Copy link
Contributor

@covert8 covert8 commented Jul 7, 2023

Description of the change

Adds supporting functions for validating ipv6 and replace strict ipv4 validator allowing both ipv4 and ipv6

Benefits

fixes #21223

Possible drawbacks

none

Applicable issues

#21223

  • fixes #

Additional information

@github-actions github-actions bot added the triage Triage is needed label Jul 7, 2023
@carrodher carrodher added the verify Execute verification workflow for these changes label Jul 9, 2023
@github-actions github-actions bot added in-progress and removed triage Triage is needed labels Jul 9, 2023
@bitnami-bot bitnami-bot removed the request for review from javsalgar July 9, 2023 06:22
@gongomgra
Copy link
Contributor

Hi @covert8,

Thanks for your contribution. I have tested your validate_ipv6 function with a little script and I think the logic is missing some cases, as it returns 1 (i.e. false) for some valid IPv6 addresses like the localhost (::1). Can you check it?

#!/bin/bash

## Test script ##

########################
# Validate if the provided argument is a valid IPv6 address
# Arguments:
#   $1 - IP to validate
# Returns:
#   Boolean
#########################
validate_ipv6() {
    local ip="${1:?ip is missing}"
    local stat=1

    if [[ $ip =~ ^[0-9a-fA-F]{1,4}(:[0-9a-fA-F]{1,4}){7}$ ]]; then
        read -r -a ip_array <<< "$(IFS=':' read -ra arr <<< "$ip" && echo "${arr[@]}")"
        for segment in "${ip_array[@]}"; do
            if [[ $segment =~ ^[0-9a-fA-F]{1,4}$ && $((16#$segment)) -le 65535 ]]; then
                continue
            else
                stat=1
                break
            fi
        done
        stat=0
    fi

    echo $stat
}

echo "127.0.0.1 - $(validate_ipv6 "127.0.0.1")"
echo "::1 - $(validate_ipv6 "::1")"
echo "2001:db8:3333:4444:5555:6666:7777:8888 - $(validate_ipv6 "2001:db8:3333:4444:5555:6666:7777:8888")"
echo "2001:db8:3333:4444:5555:6666:7777:8888:foo:bar - $(validate_ipv6 "2001:db8:3333:4444:5555:6666:7777:8888:foo:bar")"
echo "::1234:5678 - $(validate_ipv6 "::1234:5678")"

And it execution results

$ bash test.sh
127.0.0.1 - 1
::1 - 1
2001:db8:3333:4444:5555:6666:7777:8888 - 0
2001:db8:3333:4444:5555:6666:7777:8888:foo:bar - 1
::1234:5678 - 1

gongomgra
gongomgra requested changes Jul 14, 2023
View reviewed changes
Copy link
Contributor

@gongomgra gongomgra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check the IPv6 output for ::1 (IPv6 localhost address) and other special addresses like that.

@covert8
Copy link
Contributor Author

covert8 commented Jul 14, 2023 •
edited
Loading 

#!/bin/bash
validate_ipv6() {
    local ip="${1:?ip is missing}"
    local stat=1

    local full_address_regex='^([0-9a-fA-F]{1,4}:){7}[0-9a-fA-F]{1,4}$'
		local short_address_regex='^((([0-9a-fA-F]{1,4}:){0,6}[0-9a-fA-F]{1,4}){0,6}::(([0-9a-fA-F]{1,4}:){0,6}[0-9a-fA-F]{1,4}){0,6})$'


    if [[ $ip =~ $full_address_regex || $ip =~ $short_address_regex || $ip == "::" ]]; then
        stat=0
    fi

    echo $stat
}

echo "Tests for IPv6 addresses:"
echo "-------------------------"

valid_ipv6_addresses=(
    "::1"
    "::"
    "1234::"
    "2001:0db8:85a3:0000:0000:8a2e:0370:7334"
    "2001:db8::1:0:0:1"
    "fe80::1"
    "2001:0:3238:DFE1:63::FEFB"
    "2001:db8:85a3:0:0:8a2e:370:7334"
    "2001:db8:1234:5678::abcd"
    "2001:0db8:0001:0000:0000:0ab9:C0A8:0102"
    "2001:db8:0:1:1:1:1:1"
    "::1234:5678"
    "2001:0db8:0000:0000:0000:0000:0000:0000"
    "2001:0db8:0000:0000:0000::"
    "2001:0db8:0000:0000::"
    "::2001:0db8:0000:0000:0000"
    )

invalid_ipv6_addresses=(
		"::2001:0db8:0000:0000::"
    "::2001:0db8:0000::"
    "::2001:0db8::"

    "127.0.0.1"
    "192.168.0.1"
    "2001:db8:3333:4444:5555:6666:7777"
    "2001:db8:3333:4444:5555:6666:7777:"
    ":::"
    "::gggg"
    "2001:::8888"
    "2001:db8:3333:4444:5555:6666:7777:8888:9999"
    "2001:db8:3333:4444:5555:6666:7777:::"
    "2001:0db8:0000:0000:0000:0000:0000:0000:0000"
    ":::1234:5678"
)

# Test valid IPv6 addresses
echo "Valid IPv6 addresses:"
for ip in "${valid_ipv6_addresses[@]}"; do
    result=$(validate_ipv6 "$ip")
    if [[ $result -ne 0 ]]; then
        echo "$ip - Invalid"
				else
					echo "$ip - Valid"
    fi
done

# Test invalid IPv6 addresses
echo "Invalid IPv6 addresses:"
for ip in "${invalid_ipv6_addresses[@]}"; do
    result=$(validate_ipv6 "$ip")
    if [[ $result -eq 0 ]]; then
        echo "$ip - Valid"
		else
				echo "$ip - Invalid"
		fi
done

@covert8 covert8 force-pushed the patch-2 branch 2 times, most recently from 3a359c8 to 6a13033 Compare July 14, 2023 09:52
@covert8
Copy link
Contributor Author

covert8 commented Jul 14, 2023

It should cover most base cases now; although i don't know if for example 2001:0db8:0000:0000:0000:: is strictly rfc compliant

@covert8 covert8 force-pushed the patch-2 branch 2 times, most recently from eb5d82e to 9957327 Compare July 14, 2023 14:24
@covert8 covert8 requested a review from gongomgra July 18, 2023 07:24
@gongomgra gongomgra changed the title [Keycloak] Add support for ipv6 [bitnami/keycloak] Add support for ipv6 Jul 18, 2023
gongomgra
gongomgra requested changes Jul 18, 2023
View reviewed changes
Copy link
Contributor

@gongomgra gongomgra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple of aesthetic changes.

bitnami/keycloak/21/debian-11/prebuildfs/opt/bitnami/scripts/libvalidations.sh Outdated Show resolved Hide resolved
bitnami/keycloak/21/debian-11/prebuildfs/opt/bitnami/scripts/libvalidations.sh Outdated Show resolved Hide resolved
@covert8
Copy link
Contributor Author

covert8 commented Jul 18, 2023

Thanks for the review, I think I got all the changes.

@covert8 covert8 requested a review from gongomgra July 18, 2023 13:16
gongomgra
gongomgra approved these changes Jul 18, 2023
View reviewed changes
Copy link
Contributor

@gongomgra gongomgra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution!

@gongomgra
Copy link
Contributor

@covert8 changes look fine to me, but you have to meet the DCO requirement. Please check the docs linked below

https://github.com/bitnami/containers/pull/40224/checks?check_run_id=15133446058

covert8 and others added 5 commits July 18, 2023 19:32
@covert8
Add validate_ipv6 function for keycloak
f4e982e 
ip="2001:0db8:85a3:0000:0000:8a2e:0370:7334"
ip="fd00::"
ip="::"
validate_ipv6 "$ip"
result=$?

if [[ $result -eq 0 ]]; then
    echo "IPv6 address is valid"
else
    echo "IPv6 address is invalid"
fi

Signed-off-by: Louis <covert8@users.noreply.github.com>
Signed-off-by: covert <covert8@users.noreply.github.com>
@covert8
Add combinatory fuction for validate ipv4 & ipv6
58d3407 
Signed-off-by: Louis <covert8@users.noreply.github.com>
Signed-off-by: covert <covert8@users.noreply.github.com>
@covert8
Use combinated validate_ip function for keycloak envVars check
b9c29a4 
Signed-off-by: Louis <covert8@users.noreply.github.com>
Signed-off-by: covert <covert8@users.noreply.github.com>
@covert8 @dalbani
Fix return error description validate_ip helper function
50b9a4c 
Co-authored-by: Damiano Albani <damiano.albani@gmail.com>
Signed-off-by: Louis <covert8@users.noreply.github.com>
Signed-off-by: covert <covert8@users.noreply.github.com>
@covert8
Small syntax fixes
0a0d5ba 
Signed-off-by: covert <covert8@users.noreply.github.com>
@covert8
Copy link
Contributor Author

covert8 commented Jul 18, 2023

@gongomgra done :)

@covert8
Copy link
Contributor Author

covert8 commented Jul 18, 2023

Quick question, will this change also be merged for all other containers making use of validate_ipv4?

@gongomgra
Copy link
Contributor

Hi @covert8,

The changes in libvalidation.sh will be included in the rest of containers once our automated pipeline releases a new version or revision of them. The changes for using validate_ip instead of validate_ipv4 only apply to Keycloak.

@gongomgra gongomgra merged commit fcc5307 into bitnami:main Jul 19, 2023
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dalbani dalbani dalbani left review comments

@gongomgra gongomgra gongomgra approved these changes

Assignees

@gongomgra gongomgra

Labels
keycloak solved verify Execute verification workflow for these changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

IPv6 support for Keycloak
5 participants
@covert8 @gongomgra @dalbani @javsalgar @carrodher